Skip to content

Phase 5: Integration tests, edge cases, linting, final polish - #6

Merged
padak merged 1 commit into
mainfrom
phase-5-tests-final
Feb 26, 2026
Merged

Phase 5: Integration tests, edge cases, linting, final polish#6
padak merged 1 commit into
mainfrom
phase-5-tests-final

Conversation

@padak

@padak padak commented Feb 26, 2026

Copy link
Copy Markdown
Member

Implementation Summary

  • Integration test suite (tests/test_integration.py) with full workflow test (skipped without env vars)
  • Edge case tests for client: malformed JSON, empty responses, large responses, 404, URL normalization
  • Edge case tests for config store: corrupted files, missing directories, permission denied, multiple cycles
  • Additional CLI tests: help output for all commands, verbose flag, missing required args, token re-verification
  • Ruff linting (ruff>=0.8) and formatting configuration in pyproject.toml
  • Fixed all linting issues: unused imports/variables, raise ... from in except clauses, modern type annotations
  • Hardened config_store.py to handle non-dict JSON and binary file content gracefully

Changes

  • pyproject.toml: Added [tool.ruff], [tool.ruff.lint], [tool.ruff.lint.isort] configuration, ruff>=0.8 dev dependency, pytest integration marker
  • tests/test_integration.py (NEW): 3 integration tests with @pytest.mark.skipif when KBA_TEST_TOKEN_AWS not set
  • tests/test_client.py: +10 edge case tests (malformed JSON, empty response, large response, 404, URL normalization)
  • tests/test_config_store.py: +12 edge case tests (corrupted files, missing dirs, permissions, save/load cycles)
  • tests/test_cli.py: +13 tests (help, verbose, missing args, token re-verification)
  • src/keboola_agent_cli/config_store.py: Handle non-dict JSON and UnicodeDecodeError gracefully
  • src/keboola_agent_cli/client.py: Removed unused last_exception variable
  • Source files: Ruff formatting + raise ... from None for B904 compliance, unused import cleanup

Acceptance Criteria

  • All tests pass (209 passed, 3 skipped)
  • Ruff check clean (All checks passed!)
  • Ruff format clean (26 files already formatted)
  • Integration tests (skipped without env vars)
  • Edge case coverage improved
  • All 173 original tests still pass

Tests

uv run pytest tests/ -v          # 209 passed, 3 skipped
uv run ruff check src/ tests/    # All checks passed!
uv run ruff format --check src/ tests/  # 26 files already formatted

🤖 Generated with Claude Code

Add comprehensive test coverage and code quality tooling:
- Integration test suite (tests/test_integration.py) with full workflow
  test that exercises add/list/status/config-list/doctor/remove cycle,
  skipped without KBA_TEST_TOKEN_AWS env var
- Edge case tests for client: malformed JSON, empty responses, large
  responses, 404 handling, stack URL normalization
- Edge case tests for config_store: corrupted files (binary, truncated,
  null JSON, array), missing directories, permission denied, multiple
  save/load cycles
- Additional CLI tests: help output, verbose flag, missing required args,
  token re-verification on edit
- Ruff linting and formatting configuration with ruff>=0.8 dev dependency
- Fixed all linting issues: unused imports, unused variables, raise-from
  in except clauses, modern type annotations
- Hardened config_store to handle non-dict JSON and binary file content
- All 209 tests pass (173 original + 36 new), 3 integration tests skipped

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@padak
padak merged commit 9de2386 into main Feb 26, 2026
@padak
padak deleted the phase-5-tests-final branch March 2, 2026 23:05
Matovidlo added a commit that referenced this pull request Jun 15, 2026
…, dedup

Thermo-nuclear review follow-ups (behavior-preserving):
- #1/#6: move inline nfpm loop -> build/package/linux/build_packages.sh and the
  zip+sha256 -> build/package/zip_binary.sh (explicit per-OS archiver); workflow
  is now pure orchestration.
- #2: .github/actions/setup-build composite (uv + optional Node) replaces the
  repeated setup preamble in pypi/freeze.
- #3: index.sh deb/rpm/apk collapsed into a publish_repo() helper + per-format indexers.
- #5: sign.sh parses the Azure token with jq instead of a sed regex.
- #7: nfpm.yaml notes it's rendered via envsubst.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Matovidlo added a commit that referenced this pull request Jun 17, 2026
…, dedup

Thermo-nuclear review follow-ups (behavior-preserving):
- #1/#6: move inline nfpm loop -> build/package/linux/build_packages.sh and the
  zip+sha256 -> build/package/zip_binary.sh (explicit per-OS archiver); workflow
  is now pure orchestration.
- #2: .github/actions/setup-build composite (uv + optional Node) replaces the
  repeated setup preamble in pypi/freeze.
- #3: index.sh deb/rpm/apk collapsed into a publish_repo() helper + per-format indexers.
- #5: sign.sh parses the Azure token with jq instead of a sed regex.
- #7: nfpm.yaml notes it's rendered via envsubst.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
martinsifra added a commit that referenced this pull request Aug 26, 2026
The big one (#2, upgraded): the diff sides' wire shape was WRONG in the
implementation and in the test fixtures that defined it. Verified against
connection (ConfigurationVersionResponse + ConfigurationDiffData OA
schemas, now recorded in the notes wire-truth table): each side is
{version, isDeleted, diff: {name, description, changeDescription,
isDisabled, configuration, rows}} -- content NESTED under diff,
version/deletion as side metadata. The classification and both take modes
now read the envelope; the flat-side code would have been dead on arrival
against the live API.

#1: resolve_conflict no longer takes branch_id -- it derives the branch
from the MR itself (branches.branchFromId), so the conflict-set guard and
the branch being written to can never disagree; a caller-supplied id could
point the REPLACing rebase at an unrelated dev branch the guard never
checked. A published/canceled MR (null branchFromId) is refused readably.

#3: take=theirs of a deleted side collapses to the delete resolution,
symmetric with ours ('production deleted it, dev changed it' is a live
conflict shape).
#4: deletion surfaces as top-level ours_deleted/theirs_deleted booleans on
get_config_diff (None = side never existed) -- it is side metadata, not a
content path.
#5: a 'both' row where the sides agree on the identical value carries
agreed: true -- agreement, not a conflict hotspot.
#2 (message half): a take side missing required envelope keys is reported
as a backend contract violation pointing at the resolved-body workaround,
not as caller error.
#6: wire ids compared via _same_id / int-coerced (find_merge_request_for_
branch, merge()'s was_active) -- a string-serialized branchFromId can no
longer silently defeat the post-merge cleanup.
#7: the feature pre-flight raises FeatureNotEnabledError carrying the new
ErrorCode.FEATURE_NOT_ENABLED (value matches the string SearchService
already emits; categorized 'configuration' like PAYG_NOT_AVAILABLE).
#8: ConfigError imported from ..errors like every other service; the
isDefault scan hoisted to services.base.find_default_branch_id and the
copies in config/sync/workspace services migrated (lib.py keeps its own
loop -- the SDK facade does not import the services layer); verify_token
is skipped when the server already serialized viewer (the polyfill's cost
dies with the polyfill); list --state validates against the closed
vocabulary instead of returning a silent count: 0 on a typo.
#9: test imports hoisted (no mid-file noqa), mocks spec'd at the L3 seam
(KeboolaClient + MergeRequests -- a renamed L3 method now fails the tests),
and regression tests added for every finding (82 tests total).

Doc drift: layer2/layer3 references updated to BRANCHES_MERGE_REQUESTS_
FEATURE, the layer3 open nit closed, tokens.py line ref fixed.

Review: tasks/pr-703-review.md (2026-08-27).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant